-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow disabling Reaper auth #1163
Allow disabling Reaper auth #1163
Conversation
8e6a8d0
to
deec5ae
Compare
…rSecretRef.Name to "".
deec5ae
to
c33ee08
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1163 +/- ##
==========================================
+ Coverage 56.98% 57.06% +0.08%
==========================================
Files 101 101
Lines 10419 10428 +9
==========================================
+ Hits 5937 5951 +14
+ Misses 3960 3958 -2
+ Partials 522 519 -3
|
@Miles-Garnsey, FYI I moved this PR to draft since CI is failing and there seems to be a little more work before it can be reviewed. |
I'm hoping these are just flakes actually. I've seen flakes on all the failing tests in the last few weeks. (EDIT: nope, there's a problem...) |
OK, got tests passing locally, still some manual testing to do if CI goes green for us. |
Still testing this, there is some weirdness where the secrets don't appear to be deleted after the cluster is deleted. That's confusing things a little in the case where auth is toggled off and on (the only test still failing is around this case). |
280595f
to
39b041f
Compare
39b041f
to
f33b8fb
Compare
I've manually tested this and it all appears to work now. Ready for review subject to the CI run passing. What I tested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo: Add a changelog entry.
Just two small things to do and you're good for a merge.
97236bd
to
7fed28d
Compare
Quality Gate failedFailed conditions 5.9% Duplication on New Code (required ≤ 3%) |
if kc.Spec.Reaper.UiUserSecretRef == nil { | ||
uiUserSecretRef.Name = reaper.DefaultUiSecretName(kc.SanitizedName()) | ||
} | ||
kcKey := utils.GetKey(kc) | ||
if err := secret.ReconcileSecret(ctx, r.Client, cassandraUserSecretRef.Name, kcKey); err != nil { | ||
logger.Error(err, "Failed to reconcile Reaper CQL user secret", "ReaperCassandraUserSecretRef", cassandraUserSecretRef) | ||
return result.Error(err) | ||
} | ||
if err := secret.ReconcileSecret(ctx, r.Client, uiUserSecretRef.Name, kcKey); err != nil { | ||
logger.Error(err, "Failed to reconcile Reaper UI secret", "ReaperUiUserSecretRef", uiUserSecretRef) | ||
return result.Error(err) | ||
if kc.Spec.Reaper.UiUserSecretRef == nil || kc.Spec.Reaper.UiUserSecretRef.Name != "" { | ||
if err := secret.ReconcileSecret(ctx, r.Client, uiUserSecretRef.Name, kcKey); err != nil { | ||
logger.Error(err, "Failed to reconcile Reaper UI secret", "ReaperUiUserSecretRef", uiUserSecretRef) | ||
return result.Error(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @Miles-Garnsey @adejanovski ,
Isn't this change backward incompatible?
I just opened a GitHub issue #1269
Allow a user to disable Reaper auth by specifically setting the UiUserSecretRef.Name to "".
Fixes #1160